Skip to content

PITCHES - CRUD and everything else#125

Open
aamogh16 wants to merge 6 commits intomainfrom
amogh/BE-96
Open

PITCHES - CRUD and everything else#125
aamogh16 wants to merge 6 commits intomainfrom
amogh/BE-96

Conversation

@aamogh16
Copy link
Contributor

@aamogh16 aamogh16 commented Feb 5, 2026

Description

How has this been tested?

Checklist

  • I have self-reviewed my code for readability, maintainability, performance, and added comments/documentation where necessary.
  • New and existing tests pass locally with my changes.
  • I have followed the project's coding standards and best practices.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 5, 2026

📝 Walkthrough

Walkthrough

This PR implements a complete pitch management system for trips, adding database schema, backend CRUD operations with S3 integration, HTTP endpoints, and frontend React Query hooks for create, read, update, delete, and list operations. Concurrently, it consolidates and refactors existing API hook implementations across comments, memberships, files, trips, and users with streamlined type signatures.

Changes

Cohort / File(s) Summary
Database & Models
backend/internal/migrations/20260205120000_create_trip_pitches.sql, backend/internal/models/pitches.go
Adds trip_pitches table with foreign keys and indexes; introduces TripPitch ORM model and request/response DTOs with validation tags and cursor-based pagination support.
Backend Repository & Service
backend/internal/repository/pitches.go, backend/internal/repository/repository.go, backend/internal/services/pitches.go
Implements PitchRepository with Create, FindByIDAndTripID, FindByTripIDWithCursor, Update, Delete; PitchService with S3 presigning for upload/download URLs and membership validation; integrates PresignClient and URL expiration configuration.
Backend Controller & Routing
backend/internal/controllers/pitches.go, backend/internal/server/routers/trips.go
Adds PitchController with CreatePitch, ListPitches, GetPitch, UpdatePitch, DeletePitch methods; wires routes under /api/v1/trips/:tripID/pitches with service injection.
Backend Tests & Mocks
backend/internal/tests/pitch_test.go, backend/internal/tests/mocks/mock_S3PresignClient.go
Comprehensive test coverage for pitch CRUD, error handling, and membership validation; minor import alias removal in mock.
API Documentation
backend/docs/docs.go, backend/docs/swagger.json, backend/docs/swagger.yaml
Documents new pitch endpoints (GET/POST /pitches, GET/PATCH/DELETE /pitches/:pitchID) with request/response schemas, error codes, and presigned URL handling.
Frontend Pitch API
frontend/api/pitches/*.ts, frontend/api/index.ts
Introduces complete React Query hooks for pitch operations: createPitch, deletePitch, getPitch (with Suspense variant), listPitches (with Suspense variant), updatePitch; each with mutation keys, options, and typed error handling.
Frontend Type Schemas
frontend/schemas/modelsCreatePitchRequest.json, frontend/schemas/modelsPitchAPIResponse.json, frontend/schemas/modelsPitchCursorPageResult.json, frontend/schemas/modelsCreatePitchResponse.json, frontend/schemas/modelsUpdatePitchRequest.json, frontend/types/schema.gen.ts
Adds JSON Schema definitions for pitch request/response models; updates generated TypeScript schemas with pitch types and refactors existing type exports to use quoted property keys.
Frontend API Refactoring
frontend/api/comments/*.ts, frontend/api/memberships/*.ts, frontend/api/files/*.ts, frontend/api/trips/*.ts, frontend/api/users/*.ts, frontend/api/notifications/*.ts, frontend/schemas/models*.json
Consolidates mutation/query option signatures by removing generic TContext parameters from option functions; aligns options object structures to flatten client/QueryClient handling; minifies JSON schemas from multi-line to single-line format.
Frontend Index Exports
frontend/index.ts
Re-exports new pitch API surface and consolidates pitch hook exports alongside existing trip/user/comment/membership APIs.

Sequence Diagram

sequenceDiagram
    participant Client as Frontend Client
    participant Controller as PitchController
    participant Service as PitchService
    participant Presign as S3PresignClient
    participant Repo as PitchRepository
    participant DB as Database
    participant S3 as S3

    Client->>Controller: POST /trips/:tripID/pitches (CreatePitchRequest)
    Controller->>Service: Create(tripID, userID, request)
    Service->>Repo: FindByID (check membership)
    Repo->>DB: Query membership
    DB-->>Repo: membership exists
    Service->>Service: Generate pitch ID & audio key
    Service->>Repo: Create(TripPitch)
    Repo->>DB: INSERT trip_pitches
    DB-->>Repo: Created pitch
    Service->>Presign: PresignPUT(s3Key, contentType)
    Presign->>S3: Generate presigned URL
    S3-->>Presign: Upload URL (15 min expiry)
    Presign-->>Service: Upload URL
    Service-->>Controller: CreatePitchResponse (uploadURL, pitch, expiresAt)
    Controller-->>Client: 201 Created

    Client->>Controller: GET /trips/:tripID/pitches (with cursor, limit)
    Controller->>Service: List(tripID, limit, cursor)
    Service->>Repo: FindByTripIDWithCursor(tripID, limit, cursor)
    Repo->>DB: SELECT with cursor ordering
    DB-->>Repo: [Pitches], nextCursor
    loop For each pitch
        Service->>Presign: PresignGET(audioKey)
        Presign->>S3: Generate presigned URL
        S3-->>Presign: Download URL
        Presign-->>Service: Download URL
        Service->>Service: mapToPitchAPIResponse(pitch, downloadURL)
    end
    Service-->>Controller: PitchCursorPageResult
    Controller-->>Client: 200 OK
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

The PR introduces a new domain feature spanning database, backend services, and frontend integration with S3 presigning complexity. While individual patterns are familiar, the breadth requires verification across repository cursor pagination, presigned URL generation and expiration, membership authorization checks, and frontend React Query hook consistency. The frontend refactoring across multiple API modules, though largely mechanical formatting, adds volume and requires spot-checking type signature changes.

Possibly Related Issues

  • The implementation directly fulfills the pitch CRUD feature requirements, establishing the complete lifecycle for trip audio pitches from database persistence through S3 media handling to frontend consumption.

Possibly Related PRs

  • PR #108 applies similar generic signature consolidations to mutation option helpers (sendBulkNotificationMutationOptions, sendNotificationMutationOptions), suggesting alignment with an ongoing refactoring pattern for API hook standardization.

Suggested Reviewers

  • aahiltn
  • in-mai-space

🎵 A pitch endpoint takes flight,
With presigned URLs shining bright,
Cursor pagination in place,
Frontend hooks set at a pace,
Audio records find their grace! 🎙️

🚥 Pre-merge checks | ✅ 1 | ❌ 3
❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 65.42% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection ⚠️ Warning ❌ Merge conflicts detected (104 files):

⚔️ backend/cmd/main.go (content)
⚔️ backend/docs/docs.go (content)
⚔️ backend/docs/swagger.json (content)
⚔️ backend/docs/swagger.yaml (content)
⚔️ backend/go.mod (content)
⚔️ backend/go.sum (content)
⚔️ backend/internal/config/config.go (content)
⚔️ backend/internal/controllers/membership.go (content)
⚔️ backend/internal/controllers/trips.go (content)
⚔️ backend/internal/repository/repository.go (content)
⚔️ backend/internal/server/routers/membership.go (content)
⚔️ backend/internal/server/routers/routers.go (content)
⚔️ backend/internal/server/routers/trips.go (content)
⚔️ backend/internal/services/membership.go (content)
⚔️ backend/internal/services/trips.go (content)
⚔️ frontend/__tests__/utilities/images.test.ts (content)
⚔️ frontend/api/comments/index.ts (content)
⚔️ frontend/api/comments/useCreateComment.ts (content)
⚔️ frontend/api/comments/useDeleteComment.ts (content)
⚔️ frontend/api/comments/useGetPaginatedComments.ts (content)
⚔️ frontend/api/comments/useGetPaginatedCommentsSuspense.ts (content)
⚔️ frontend/api/comments/useUpdateComment.ts (content)
⚔️ frontend/api/example/index.ts (content)
⚔️ frontend/api/example/useHealthcheck.ts (content)
⚔️ frontend/api/example/useHealthcheckSuspense.ts (content)
⚔️ frontend/api/files/index.ts (content)
⚔️ frontend/api/files/useCheckS3Health.ts (content)
⚔️ frontend/api/files/useCheckS3HealthSuspense.ts (content)
⚔️ frontend/api/files/useConfirmUpload.ts (content)
⚔️ frontend/api/files/useCreateUploadURLs.ts (content)
⚔️ frontend/api/files/useGetFile.ts (content)
⚔️ frontend/api/files/useGetFileAllSizes.ts (content)
⚔️ frontend/api/files/useGetFileAllSizesSuspense.ts (content)
⚔️ frontend/api/files/useGetFileSuspense.ts (content)
⚔️ frontend/api/index.ts (content)
⚔️ frontend/api/memberships/index.ts (content)
⚔️ frontend/api/memberships/useAddMember.ts (content)
⚔️ frontend/api/memberships/useDemoteFromAdmin.ts (content)
⚔️ frontend/api/memberships/useGetMembership.ts (content)
⚔️ frontend/api/memberships/useGetMembershipSuspense.ts (content)
⚔️ frontend/api/memberships/useGetTripMembers.ts (content)
⚔️ frontend/api/memberships/useGetTripMembersSuspense.ts (content)
⚔️ frontend/api/memberships/usePromoteToAdmin.ts (content)
⚔️ frontend/api/memberships/useRemoveMember.ts (content)
⚔️ frontend/api/memberships/useUpdateMembership.ts (content)
⚔️ frontend/api/notifications/index.ts (content)
⚔️ frontend/api/notifications/useSendBulkNotification.ts (content)
⚔️ frontend/api/notifications/useSendNotification.ts (content)
⚔️ frontend/api/trips/index.ts (content)
⚔️ frontend/api/trips/useCreateTrip.ts (content)
⚔️ frontend/api/trips/useDeleteTrip.ts (content)
⚔️ frontend/api/trips/useGetAllTrips.ts (content)
⚔️ frontend/api/trips/useGetAllTripsSuspense.ts (content)
⚔️ frontend/api/trips/useGetTrip.ts (content)
⚔️ frontend/api/trips/useGetTripSuspense.ts (content)
⚔️ frontend/api/trips/useUpdateTrip.ts (content)
⚔️ frontend/api/users/index.ts (content)
⚔️ frontend/api/users/useCreateUser.ts (content)
⚔️ frontend/api/users/useDeleteUser.ts (content)
⚔️ frontend/api/users/useGetCurrentUser.ts (content)
⚔️ frontend/api/users/useGetCurrentUserSuspense.ts (content)
⚔️ frontend/api/users/useGetUser.ts (content)
⚔️ frontend/api/users/useGetUserSuspense.ts (content)
⚔️ frontend/api/users/useUpdateUser.ts (content)
⚔️ frontend/app/(app)/_layout.tsx (content)
⚔️ frontend/app/(app)/index.tsx (content)
⚔️ frontend/app/(app)/test-realtime.tsx (content)
⚔️ frontend/contexts/notification.tsx (content)
⚔️ frontend/index.ts (content)
⚔️ frontend/schemas/errsAPIError.json (content)
⚔️ frontend/schemas/modelsComment.json (content)
⚔️ frontend/schemas/modelsCommentAPIResponse.json (content)
⚔️ frontend/schemas/modelsConfirmUploadRequest.json (content)
⚔️ frontend/schemas/modelsConfirmUploadResponse.json (content)
⚔️ frontend/schemas/modelsCreateCommentRequest.json (content)
⚔️ frontend/schemas/modelsCreateMembershipRequest.json (content)
⚔️ frontend/schemas/modelsCreateTripRequest.json (content)
⚔️ frontend/schemas/modelsCreateUserRequest.json (content)
⚔️ frontend/schemas/modelsEntityType.json (content)
⚔️ frontend/schemas/modelsGetFileAllSizesResponse.json (content)
⚔️ frontend/schemas/modelsGetFileResponse.json (content)
⚔️ frontend/schemas/modelsImageSize.json (content)
⚔️ frontend/schemas/modelsMembership.json (content)
⚔️ frontend/schemas/modelsMembershipAPIResponse.json (content)
⚔️ frontend/schemas/modelsMembershipCursorPageResult.json (content)
⚔️ frontend/schemas/modelsNotificationError.json (content)
⚔️ frontend/schemas/modelsNotificationResponse.json (content)
⚔️ frontend/schemas/modelsPaginatedCommentsResponse.json (content)
⚔️ frontend/schemas/modelsS3HealthCheckResponse.json (content)
⚔️ frontend/schemas/modelsSendBulkNotificationRequest.json (content)
⚔️ frontend/schemas/modelsSendNotificationRequest.json (content)
⚔️ frontend/schemas/modelsSizedUploadURL.json (content)
⚔️ frontend/schemas/modelsTrip.json (content)
⚔️ frontend/schemas/modelsTripAPIResponse.json (content)
⚔️ frontend/schemas/modelsTripCursorPageResult.json (content)
⚔️ frontend/schemas/modelsUpdateCommentRequest.json (content)
⚔️ frontend/schemas/modelsUpdateMembershipRequest.json (content)
⚔️ frontend/schemas/modelsUpdateTripRequest.json (content)
⚔️ frontend/schemas/modelsUpdateUserRequest.json (content)
⚔️ frontend/schemas/modelsUploadURLRequest.json (content)
⚔️ frontend/schemas/modelsUploadURLResponse.json (content)
⚔️ frontend/schemas/modelsUser.json (content)
⚔️ frontend/types/schema.gen.ts (content)
⚔️ frontend/types/types.gen.ts (content)

These conflicts must be resolved before merging into main.
Resolve conflicts locally and push changes to this branch.
Title check ❓ Inconclusive The title 'PITCHES - CRUD and everything else' partially describes the changeset but uses vague language ('everything else') that obscures the full scope of changes. Revise the title to be more specific. For example: 'Add pitch CRUD endpoints and frontend integration' or 'Implement trip pitches API with full lifecycle support'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch amogh/BE-96
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch amogh/BE-96
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@aamogh16 aamogh16 marked this pull request as ready for review February 14, 2026 00:23
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 16

🤖 Fix all issues with AI agents
In `@backend/docs/swagger.json`:
- Around line 1252-1374: The Swagger spec for the createPitch endpoint is
documenting a 403 Forbidden but tests and controller behavior return 404 for
non-members; update the API documentation to match runtime behavior by changing
the controller's Swagger annotation (the createPitch endpoint's `@Failure` 403
entry) to `@Failure` 404 and update the generated swagger.json responses for
operationId "createPitch" (POST /api/v1/trips/{tripID}/pitches) to remove or
replace the 403 response with a 404 response (ensure the 404 uses errs.APIError
or the appropriate schema), and scan other pitch endpoints for similar
mismatches between `@Failure` annotations and actual status codes.

In `@backend/internal/controllers/pitches.go`:
- Around line 89-109: ListPitches (and the other controllers GetByID, Update,
Delete) never extract the caller userID from c.Locals("userID") nor pass it to
the service, so membership checks (like membershipRepo.IsMember used by Create)
cannot run; modify each controller (ListPitches, GetByID, Update, Delete) to
read userID := c.Locals("userID") (validate/convert as needed), pass that userID
into the corresponding pitchService methods (e.g., pitchService.List now accepts
userID), and update the service implementations to call
membershipRepo.IsMember(userID, tripID) before performing the operation,
returning a proper authorization error when not a member.

In `@backend/internal/migrations/20260205120000_create_trip_pitches.sql`:
- Around line 35-43: The down migration unconditionally drops the shared
function update_updated_at_column(), which may be used by other tables; instead,
make the function table-specific (e.g., update_trip_pitches_updated_at) and
update both the CREATE FUNCTION and CREATE TRIGGER statements in this migration
to use that name, then change the Down block to DROP TRIGGER IF EXISTS
update_trip_pitches_updated_at ON trip_pitches; and DROP FUNCTION IF EXISTS
update_trip_pitches_updated_at(); so only the per-table trigger function is
removed; ensure any references in this file to update_updated_at_column() are
replaced with the new per-table function name.
- Around line 22-28: The migration defines a shared function
update_updated_at_column() used by multiple migrations which causes
cross-migration breakage; rename the function to a table-specific name (e.g.,
update_trip_pitches_updated_at_column()) and update the trigger definition(s) in
this migration to call the new function name, and change the down migration to
DROP FUNCTION IF EXISTS update_trip_pitches_updated_at_column(); so this
migration creates, uses, and drops its own uniquely named function without
affecting other tables' triggers.

In `@backend/internal/models/pitches.go`:
- Around line 23-27: The ContentType field on CreatePitchRequest is currently
unrestricted and must be limited to an allowlist of audio MIME types; update
validation for CreatePitchRequest.ContentType to only accept known audio MIME
types (e.g., audio/mpeg, audio/mp3, audio/wav, audio/x-wav, audio/ogg,
audio/webm, audio/aac, audio/flac) either by replacing the struct tag with a
validate:"required,oneof=..." list or by adding a dedicated validator (register
a custom "audio" validator) that checks against a central audioMIMETypes
allowlist constant and then use validate:"required,audio" on ContentType; ensure
the validator is registered where request validation is wired so
CreatePitchRequest is rejected if ContentType is not in the allowlist (reference
CreatePitchRequest.ContentType and extensionFromContentType() for context).

In `@backend/internal/repository/pitches.go`:
- Around line 76-109: The Update method can build an invalid SQL when no fields
on UpdatePitchRequest are set; add a guard at the start of
pitchRepository.Update to detect when req.Title, req.Description and
req.Duration are all nil and return early with a clear error (e.g.,
errs.ErrBadRequest or a new validation error) instead of proceeding to
r.db.NewUpdate/ upd.Exec; ensure the guard references Update,
UpdatePitchRequest, the upd variable (r.db.NewUpdate()) and returns before
attempting to execute the update so no empty SET clause is produced, then keep
the rest of the method as-is to fetch the TripPitch on success.

In `@backend/internal/server/routers/trips.go`:
- Around line 41-46: The UpdatePitch and DeletePitch handlers currently only
require TripMemberRequired and therefore allow any member to change any pitch;
mirror the CreatePitch pattern by extracting the authenticated userID in the
UpdatePitch and DeletePitch controller handlers and pass it into the pitch
service (e.g., call service.UpdatePitch(ctx, userID, tripID, pitchID, ... ) and
service.DeletePitch(ctx, userID, tripID, pitchID) or similar). In the service
(or repository) add an ownership check that loads the pitch owner (e.g.,
repo.GetPitchOwner or repo.GetPitchByID) and compares ownerID == userID,
returning a 403/permission error if they differ; only proceed to update/delete
when the user is the owner. Ensure errors are propagated back to the controller
to return the correct HTTP status.

In `@backend/internal/services/pitches.go`:
- Around line 61-104: The Create handler currently inserts the DB record via
s.pitchRepo.Create and only then calls s.presignClient.PresignPutObject, which
leaves an orphan pitch if presign fails; change the flow in Create to generate
the presigned upload URL first (call s.presignClient.PresignPutObject to get
presigned.URL and expires), then construct the models.TripPitch and call
s.pitchRepo.Create (or alternatively, if you prefer to keep the insert-first
pattern, call s.pitchRepo.Create and on any presign error call a delete/rollback
via s.pitchRepo.Delete to remove the created pitch); update code paths that
build apiPitch via pitchToAPIResponse to use the created pitch and return
UploadURL/ExpiresAt from the successful presign so no pitch exists without a
usable upload URL.
- Around line 143-151: The List loop is calling s.presignGetURL sequentially for
each pitch (items, s.presignGetURL, pitchToAPIResponse), causing latency at
scale; fix by not presigning in the list path—remove/omit audio_url from the
list response and change the list handler to call pitchToAPIResponse without
presigned URLs (or have pitchToAPIResponse accept a nil/empty audioURL), and
move presigning into the GetByID path where s.presignGetURL is invoked on
demand; alternatively, if audio_url must remain in List, parallelize presign
calls with a bounded worker pool (e.g., spawn N goroutines, use a jobs/results
channel and context cancellation) so s.presignGetURL calls run concurrently but
limited to a safe concurrency level.
- Around line 46-58: NewPitchService currently doesn't validate required config
fields which leads to nil-pointer panics; modify NewPitchService to validate
PitchServiceConfig.PresignClient, .PitchRepo, .MembershipRepo and .BucketName
(non-nil and non-empty for BucketName) and return a clear error for
missing/invalid values instead of constructing a service with nil
dependencies—this will require changing the constructor signature to return
(PitchServiceInterface, error) and returning a descriptive error for each
missing dependency (referencing NewPitchService, PitchServiceConfig, and the
PitchService struct/presignClient, pitchRepo, membershipRepo, bucketName
fields).
- Around line 184-186: The Delete method (PitchService.Delete) currently only
calls s.pitchRepo.Delete and leaves the S3 audio object orphaned; change it to
first load the pitch (e.g., via s.pitchRepo.Get or similar) to obtain the
AudioS3Key, then call the standard S3 client's DeleteObject for that key before
deleting the DB record (or delete DB record only after successful S3 deletion),
handling and returning errors appropriately; if the service only has a presign
client, add a standard S3 client to PitchServiceConfig and use it here, and
ensure you handle missing AudioS3Key gracefully to avoid panics.
- Around line 214-229: The function extensionFromContentType currently returns
"m4a" for any unrecognized MIME type; change its signature to return (string,
error) and make it explicitly validate against an allowlist of known audio MIME
types (e.g., "audio/m4a", "audio/mp4", "audio/mpeg", "audio/mp3", "audio/wav",
"audio/ogg", "audio/webm"); for matches return the proper extension, otherwise
return a non-nil error indicating unsupported content type. Update all callers
of extensionFromContentType to handle the error (propagate or reject the upload)
so invalid types like "application/pdf" no longer silently become .m4a. Ensure
error messages include the original contentType value for easier diagnosis.
- Around line 107-118: GetByID, List, Update, and Delete currently lack a userID
parameter and thus skip authorization: update the service method signatures for
GetByID, List, Update, and Delete to accept a userID (same type used by Create),
have the controller extract and pass the userID, and add authorization checks
inside each service method (for GetByID/List call trip membership verification
before calling pitchRepo.FindByIDAndTripID or the list repo method; for
Update/Delete verify that the user is either the pitch owner or a trip admin
before proceeding to modify/delete; for Update ensure ownership/admin check
precedes any update logic and for GetByID include the membership check before
presignGetURL and pitchToAPIResponse). Ensure existing helper methods such as
presignGetURL and pitchToAPIResponse are used only after authorization succeeds.

In `@backend/internal/tests/pitch_test.go`:
- Around line 304-340: TestPitchDelete is missing a case that verifies
non-members are rejected; add a subtest under TestPitchDelete that creates a
separate non-member user (e.g., nonMemberID via createTestUser), reuses
createPitch to create pitchID, then issues a DELETE request to
"/api/v1/trips/"+tripID+"/pitches/"+pitchID using testkit.Request with UserID
set to &nonMemberID and AssertStatus(http.StatusNotFound); this mirrors the
Create test pattern and ensures the delete endpoint is scoped to trip members.
- Around line 148-200: In TestPitchList's "returns pitches with audio_url"
subtest remove the redundant requireS3 call: the helper createPitch(t, app,
userID, tripID, "P1", "audio/mpeg") already invokes requireS3 internally, so
delete the extra requireS3(t) invocation to avoid reloading S3 configuration
unnecessarily and keep the rest of the subtest (the subsequent testkit request
and assertions) unchanged.

In `@frontend/api/trips/useGetAllTrips.ts`:
- Line 24: The request helper is dropping query params so pagination params
passed from useGetAllTrips (and other generated hooks) are never sent; add an
optional params property to the RequestConfig type (e.g., params?: Record<string
| number, any> | URLSearchParams) and update the request function to serialize
those params onto the fetch URL (e.g., append new
URLSearchParams(config.params).toString() to `${BASE_URL}${config.url}` only
when params exist), ensuring callers like useGetAllTrips.ts that pass params are
transmitted to the server.

Comment on lines +1252 to +1374
"/api/v1/trips/{tripID}/pitches": {
"get": {
"description": "Returns pitches for the trip with cursor-based pagination",
"produces": [
"application/json"
],
"tags": [
"pitches"
],
"summary": "Get all pitches for a trip",
"operationId": "listPitches",
"parameters": [
{
"type": "string",
"description": "Trip ID",
"name": "tripID",
"in": "path",
"required": true
},
{
"type": "integer",
"description": "Max items per page (default 20, max 100)",
"name": "limit",
"in": "query"
},
{
"type": "string",
"description": "Opaque cursor from previous response next_cursor",
"name": "cursor",
"in": "query"
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/models.PitchCursorPageResult"
}
},
"400": {
"description": "Bad Request",
"schema": {
"$ref": "#/definitions/errs.APIError"
}
},
"404": {
"description": "Not Found",
"schema": {
"$ref": "#/definitions/errs.APIError"
}
},
"500": {
"description": "Internal Server Error",
"schema": {
"$ref": "#/definitions/errs.APIError"
}
}
}
},
"post": {
"description": "Creates a new pitch for the trip and returns a presigned URL to upload the audio file",
"consumes": [
"application/json"
],
"produces": [
"application/json"
],
"tags": [
"pitches"
],
"summary": "Create a pitch",
"operationId": "createPitch",
"parameters": [
{
"type": "string",
"description": "Trip ID",
"name": "tripID",
"in": "path",
"required": true
},
{
"description": "Create pitch request",
"name": "request",
"in": "body",
"required": true,
"schema": {
"$ref": "#/definitions/models.CreatePitchRequest"
}
}
],
"responses": {
"201": {
"description": "Created",
"schema": {
"$ref": "#/definitions/models.CreatePitchResponse"
}
},
"400": {
"description": "Bad Request",
"schema": {
"$ref": "#/definitions/errs.APIError"
}
},
"403": {
"description": "Forbidden",
"schema": {
"$ref": "#/definitions/errs.APIError"
}
},
"422": {
"description": "Unprocessable Entity",
"schema": {
"$ref": "#/definitions/errs.APIError"
}
},
"500": {
"description": "Internal Server Error",
"schema": {
"$ref": "#/definitions/errs.APIError"
}
}
}
}
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Swagger documents 403 for createPitch, but the test expects 404 for non-members.

The createPitch endpoint Swagger spec lists a 403 Forbidden response (line 1355) but does not list 404. However, the test TestPitchCreate / "non-member gets 404" asserts http.StatusNotFound. The Swagger annotation in the controller (line 38: @Failure 403) should be updated to match the actual behavior.

This same issue may apply to other pitch endpoints if non-member access returns 404 rather than being documented.

🤖 Prompt for AI Agents
In `@backend/docs/swagger.json` around lines 1252 - 1374, The Swagger spec for the
createPitch endpoint is documenting a 403 Forbidden but tests and controller
behavior return 404 for non-members; update the API documentation to match
runtime behavior by changing the controller's Swagger annotation (the
createPitch endpoint's `@Failure` 403 entry) to `@Failure` 404 and update the
generated swagger.json responses for operationId "createPitch" (POST
/api/v1/trips/{tripID}/pitches) to remove or replace the 403 response with a 404
response (ensure the 404 uses errs.APIError or the appropriate schema), and scan
other pitch endpoints for similar mismatches between `@Failure` annotations and
actual status codes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aamogh16 let's add the error code in the doc just to be sure!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Comment on lines +89 to +109
func (ctrl *PitchController) ListPitches(c *fiber.Ctx) error {
tripID, err := validators.ValidateID(c.Params("tripID"))
if err != nil {
return errs.InvalidUUID()
}

var params models.GetPitchesQueryParams
if err := utilities.ParseAndValidateQueryParams(c, ctrl.validator, &params); err != nil {
return err
}
limit, cursorToken := utilities.ExtractLimitAndCursor(&params)

result, err := ctrl.pitchService.List(c.Context(), tripID, limit, cursorToken)
if err != nil {
if errors.Is(err, errs.ErrInvalidCursor) {
return errs.BadRequest(err)
}
return err
}
return c.Status(http.StatusOK).JSON(result)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 1. Find pitch route registration and middleware
echo "=== Pitch Route Registration ==="
rg -n "pitches" backend/internal/server/routers/ -A 3 -B 3

# 2. Check PitchService method signatures
echo -e "\n=== PitchService Method Signatures ==="
rg -n "func \(.* PitchService\) (List|GetByID|Update|Delete|Create)" backend/internal/services/pitches.go

# 3. Look for complete service method signatures with parameters
echo -e "\n=== PitchService Complete Methods ==="
cat backend/internal/services/pitches.go | head -200

Repository: GenerateNU/toggo

Length of output: 7597


🏁 Script executed:

#!/bin/bash
# Get the full path of pitches service to understand its structure
find backend -name "pitches.go" -type f | xargs wc -l

Repository: GenerateNU/toggo

Length of output: 246


🏁 Script executed:

#!/bin/bash
# Read the PitchService to check method signatures and authorization logic
cat -n backend/internal/services/pitches.go

Repository: GenerateNU/toggo

Length of output: 8943


🏁 Script executed:

#!/bin/bash
# Check the controller file to confirm what the review comment says
cat -n backend/internal/controllers/pitches.go | head -250

Repository: GenerateNU/toggo

Length of output: 8296


GetByID, List, Update, and Delete do not accept userID, preventing membership authorization.

The Create method accepts userID and enforces membership checks via membershipRepo.IsMember(). The four other service methods (GetByID, List, Update, Delete) do not accept userID and therefore cannot verify that the caller is a trip member. The controller layers for these operations do not extract userID from c.Locals("userID"), so no authorization is enforced before calling the service.

Pass userID through the controller and service layers for GetByID, List, Update, and Delete, and add membership checks to each method (similar to Create).

🤖 Prompt for AI Agents
In `@backend/internal/controllers/pitches.go` around lines 89 - 109, ListPitches
(and the other controllers GetByID, Update, Delete) never extract the caller
userID from c.Locals("userID") nor pass it to the service, so membership checks
(like membershipRepo.IsMember used by Create) cannot run; modify each controller
(ListPitches, GetByID, Update, Delete) to read userID := c.Locals("userID")
(validate/convert as needed), pass that userID into the corresponding
pitchService methods (e.g., pitchService.List now accepts userID), and update
the service implementations to call membershipRepo.IsMember(userID, tripID)
before performing the operation, returning a proper authorization error when not
a member.

Comment on lines +22 to +28
CREATE OR REPLACE FUNCTION update_updated_at_column()
RETURNS TRIGGER AS $$
BEGIN
NEW.updated_at = now();
RETURN NEW;
END;
$$ language 'plpgsql';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

rg -n 'update_updated_at_column' --type=sql

Repository: GenerateNU/toggo

Length of output: 1049


🏁 Script executed:

find . -name '20260205120000_create_trip_pitches.sql' -type f -exec cat -n {} \;

Repository: GenerateNU/toggo

Length of output: 1851


Shared function name update_updated_at_column() creates breaking dependency across migrations.

This function is created identically in three separate migrations: 20260123213356_create_trip.sql, 20260123213543_create_membership.sql, and 20260205120000_create_trip_pitches.sql. Each migration creates its own trigger on its respective table, but all share the same function name.

The down migration (line 38) drops the function unconditionally with DROP FUNCTION IF EXISTS update_updated_at_column(). Rolling back any one of these migrations will break the triggers on all other tables that depend on the same function.

Rename the function to be table-specific, for example update_trip_pitches_updated_at_column(), so each migration manages its own function lifecycle independently.

🤖 Prompt for AI Agents
In `@backend/internal/migrations/20260205120000_create_trip_pitches.sql` around
lines 22 - 28, The migration defines a shared function
update_updated_at_column() used by multiple migrations which causes
cross-migration breakage; rename the function to a table-specific name (e.g.,
update_trip_pitches_updated_at_column()) and update the trigger definition(s) in
this migration to call the new function name, and change the down migration to
DROP FUNCTION IF EXISTS update_trip_pitches_updated_at_column(); so this
migration creates, uses, and drops its own uniquely named function without
affecting other tables' triggers.

Comment on lines +35 to +43
-- +goose Down
-- +goose StatementBegin
DROP TRIGGER IF EXISTS update_trip_pitches_updated_at ON trip_pitches;
DROP FUNCTION IF EXISTS update_updated_at_column();
DROP INDEX IF EXISTS idx_trip_pitches_trip_created_id;
DROP INDEX IF EXISTS idx_trip_pitches_user_id;
DROP INDEX IF EXISTS idx_trip_pitches_trip_id;
DROP TABLE IF EXISTS trip_pitches;
-- +goose StatementEnd
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Down migration drops the shared function unconditionally.

Line 38 (DROP FUNCTION IF EXISTS update_updated_at_column()) will remove the function even if other tables' triggers still reference it. This is the downstream consequence of the shared function name flagged above. If you rename the function per-table, this becomes safe.

🤖 Prompt for AI Agents
In `@backend/internal/migrations/20260205120000_create_trip_pitches.sql` around
lines 35 - 43, The down migration unconditionally drops the shared function
update_updated_at_column(), which may be used by other tables; instead, make the
function table-specific (e.g., update_trip_pitches_updated_at) and update both
the CREATE FUNCTION and CREATE TRIGGER statements in this migration to use that
name, then change the Down block to DROP TRIGGER IF EXISTS
update_trip_pitches_updated_at ON trip_pitches; and DROP FUNCTION IF EXISTS
update_trip_pitches_updated_at(); so only the per-table trigger function is
removed; ensure any references in this file to update_updated_at_column() are
replaced with the new per-table function name.

Comment on lines +214 to +229
func extensionFromContentType(contentType string) string {
contentType = strings.TrimSpace(strings.ToLower(contentType))
switch {
case strings.HasSuffix(contentType, "m4a"), contentType == "audio/mp4":
return "m4a"
case strings.HasSuffix(contentType, "mpeg"), strings.HasSuffix(contentType, "mp3"):
return "mp3"
case strings.HasSuffix(contentType, "wav"):
return "wav"
case strings.HasSuffix(contentType, "ogg"):
return "ogg"
case strings.HasSuffix(contentType, "webm"):
return "webm"
default:
return "m4a"
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

extensionFromContentType silently defaults to "m4a" for unrecognized MIME types.

If an unexpected content type like application/pdf is passed, it will silently produce a .m4a extension. This could lead to files with incorrect extensions stored in S3.

Consider returning an error for unsupported content types, or validate the content type against an allowlist before reaching this function.

Proposed fix
-func extensionFromContentType(contentType string) string {
+func extensionFromContentType(contentType string) (string, error) {
 	contentType = strings.TrimSpace(strings.ToLower(contentType))
 	switch {
 	case strings.HasSuffix(contentType, "m4a"), contentType == "audio/mp4":
-		return "m4a"
+		return "m4a", nil
 	case strings.HasSuffix(contentType, "mpeg"), strings.HasSuffix(contentType, "mp3"):
-		return "mp3"
+		return "mp3", nil
 	case strings.HasSuffix(contentType, "wav"):
-		return "wav"
+		return "wav", nil
 	case strings.HasSuffix(contentType, "ogg"):
-		return "ogg"
+		return "ogg", nil
 	case strings.HasSuffix(contentType, "webm"):
-		return "webm"
+		return "webm", nil
 	default:
-		return "m4a"
+		return "", fmt.Errorf("unsupported content type: %s", contentType)
 	}
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func extensionFromContentType(contentType string) string {
contentType = strings.TrimSpace(strings.ToLower(contentType))
switch {
case strings.HasSuffix(contentType, "m4a"), contentType == "audio/mp4":
return "m4a"
case strings.HasSuffix(contentType, "mpeg"), strings.HasSuffix(contentType, "mp3"):
return "mp3"
case strings.HasSuffix(contentType, "wav"):
return "wav"
case strings.HasSuffix(contentType, "ogg"):
return "ogg"
case strings.HasSuffix(contentType, "webm"):
return "webm"
default:
return "m4a"
}
func extensionFromContentType(contentType string) (string, error) {
contentType = strings.TrimSpace(strings.ToLower(contentType))
switch {
case strings.HasSuffix(contentType, "m4a"), contentType == "audio/mp4":
return "m4a", nil
case strings.HasSuffix(contentType, "mpeg"), strings.HasSuffix(contentType, "mp3"):
return "mp3", nil
case strings.HasSuffix(contentType, "wav"):
return "wav", nil
case strings.HasSuffix(contentType, "ogg"):
return "ogg", nil
case strings.HasSuffix(contentType, "webm"):
return "webm", nil
default:
return "", fmt.Errorf("unsupported content type: %s", contentType)
}
}
🤖 Prompt for AI Agents
In `@backend/internal/services/pitches.go` around lines 214 - 229, The function
extensionFromContentType currently returns "m4a" for any unrecognized MIME type;
change its signature to return (string, error) and make it explicitly validate
against an allowlist of known audio MIME types (e.g., "audio/m4a", "audio/mp4",
"audio/mpeg", "audio/mp3", "audio/wav", "audio/ogg", "audio/webm"); for matches
return the proper extension, otherwise return a non-nil error indicating
unsupported content type. Update all callers of extensionFromContentType to
handle the error (propagate or reject the upload) so invalid types like
"application/pdf" no longer silently become .m4a. Ensure error messages include
the original contentType value for easier diagnosis.

Comment on lines +148 to +200
func TestPitchList(t *testing.T) {
app := fakes.GetSharedTestApp()
userID := createTestUser(t, app, "ListUser", fakes.GenerateRandomUsername(), fakes.GenerateRandomPhoneNumber())
tripID := createTestTrip(t, app, userID, "ListTrip", 100, 500)

t.Run("returns empty list", func(t *testing.T) {
resp := testkit.New(t).
Request(testkit.Request{
App: app,
Route: "/api/v1/trips/" + tripID + "/pitches",
Method: testkit.GET,
UserID: &userID,
}).
AssertStatus(http.StatusOK).
AssertField("limit", float64(20)).
AssertFieldExists("items").
GetBody()
items, ok := resp["items"].([]any)
require.True(t, ok)
assert.Len(t, items, 0)
})

t.Run("returns pitches with audio_url when S3 configured", func(t *testing.T) {
_, _ = createPitch(t, app, userID, tripID, "P1", "audio/mpeg")
requireS3(t)
resp := testkit.New(t).
Request(testkit.Request{
App: app,
Route: "/api/v1/trips/" + tripID + "/pitches",
Method: testkit.GET,
UserID: &userID,
}).
AssertStatus(http.StatusOK).
GetBody()
items, ok := resp["items"].([]any)
require.True(t, ok)
require.GreaterOrEqual(t, len(items), 1)
first := items[0].(map[string]any)
assert.Equal(t, "P1", first["title"])
assert.Contains(t, first, "audio_url")
})

t.Run("invalid trip ID returns 400", func(t *testing.T) {
testkit.New(t).
Request(testkit.Request{
App: app,
Route: "/api/v1/trips/bad-uuid/pitches",
Method: testkit.GET,
UserID: &userID,
}).
AssertStatus(http.StatusBadRequest)
})
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Redundant requireS3 call in "returns pitches with audio_url" subtest.

createPitch on line 171 already calls requireS3(t) internally. The second call on line 172 is redundant. Not a bug, but it loads the full configuration a second time for no reason.

Proposed fix
 	t.Run("returns pitches with audio_url when S3 configured", func(t *testing.T) {
 		_, _ = createPitch(t, app, userID, tripID, "P1", "audio/mpeg")
-		requireS3(t)
 		resp := testkit.New(t).
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func TestPitchList(t *testing.T) {
app := fakes.GetSharedTestApp()
userID := createTestUser(t, app, "ListUser", fakes.GenerateRandomUsername(), fakes.GenerateRandomPhoneNumber())
tripID := createTestTrip(t, app, userID, "ListTrip", 100, 500)
t.Run("returns empty list", func(t *testing.T) {
resp := testkit.New(t).
Request(testkit.Request{
App: app,
Route: "/api/v1/trips/" + tripID + "/pitches",
Method: testkit.GET,
UserID: &userID,
}).
AssertStatus(http.StatusOK).
AssertField("limit", float64(20)).
AssertFieldExists("items").
GetBody()
items, ok := resp["items"].([]any)
require.True(t, ok)
assert.Len(t, items, 0)
})
t.Run("returns pitches with audio_url when S3 configured", func(t *testing.T) {
_, _ = createPitch(t, app, userID, tripID, "P1", "audio/mpeg")
requireS3(t)
resp := testkit.New(t).
Request(testkit.Request{
App: app,
Route: "/api/v1/trips/" + tripID + "/pitches",
Method: testkit.GET,
UserID: &userID,
}).
AssertStatus(http.StatusOK).
GetBody()
items, ok := resp["items"].([]any)
require.True(t, ok)
require.GreaterOrEqual(t, len(items), 1)
first := items[0].(map[string]any)
assert.Equal(t, "P1", first["title"])
assert.Contains(t, first, "audio_url")
})
t.Run("invalid trip ID returns 400", func(t *testing.T) {
testkit.New(t).
Request(testkit.Request{
App: app,
Route: "/api/v1/trips/bad-uuid/pitches",
Method: testkit.GET,
UserID: &userID,
}).
AssertStatus(http.StatusBadRequest)
})
}
func TestPitchList(t *testing.T) {
app := fakes.GetSharedTestApp()
userID := createTestUser(t, app, "ListUser", fakes.GenerateRandomUsername(), fakes.GenerateRandomPhoneNumber())
tripID := createTestTrip(t, app, userID, "ListTrip", 100, 500)
t.Run("returns empty list", func(t *testing.T) {
resp := testkit.New(t).
Request(testkit.Request{
App: app,
Route: "/api/v1/trips/" + tripID + "/pitches",
Method: testkit.GET,
UserID: &userID,
}).
AssertStatus(http.StatusOK).
AssertField("limit", float64(20)).
AssertFieldExists("items").
GetBody()
items, ok := resp["items"].([]any)
require.True(t, ok)
assert.Len(t, items, 0)
})
t.Run("returns pitches with audio_url when S3 configured", func(t *testing.T) {
_, _ = createPitch(t, app, userID, tripID, "P1", "audio/mpeg")
resp := testkit.New(t).
Request(testkit.Request{
App: app,
Route: "/api/v1/trips/" + tripID + "/pitches",
Method: testkit.GET,
UserID: &userID,
}).
AssertStatus(http.StatusOK).
GetBody()
items, ok := resp["items"].([]any)
require.True(t, ok)
require.GreaterOrEqual(t, len(items), 1)
first := items[0].(map[string]any)
assert.Equal(t, "P1", first["title"])
assert.Contains(t, first, "audio_url")
})
t.Run("invalid trip ID returns 400", func(t *testing.T) {
testkit.New(t).
Request(testkit.Request{
App: app,
Route: "/api/v1/trips/bad-uuid/pitches",
Method: testkit.GET,
UserID: &userID,
}).
AssertStatus(http.StatusBadRequest)
})
}
🤖 Prompt for AI Agents
In `@backend/internal/tests/pitch_test.go` around lines 148 - 200, In
TestPitchList's "returns pitches with audio_url" subtest remove the redundant
requireS3 call: the helper createPitch(t, app, userID, tripID, "P1",
"audio/mpeg") already invokes requireS3 internally, so delete the extra
requireS3(t) invocation to avoid reloading S3 configuration unnecessarily and
keep the rest of the subtest (the subsequent testkit request and assertions)
unchanged.

export async function getAllTrips(params?: GetAllTripsQueryParams, config: Partial<RequestConfig> & { client?: typeof fetch } = {}) {
const { client: request = fetch, ...requestConfig } = config

const res = await request<GetAllTripsQueryResponse, ResponseErrorConfig<GetAllTrips400 | GetAllTrips401 | GetAllTrips500>, unknown>({ method : "GET", url : `/api/v1/trips`, params, ... requestConfig })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's find and examine the frontend/api/client.ts file
echo "=== Looking for frontend/api/client.ts ==="
fd "client.ts" frontend/api/

echo ""
echo "=== RequestConfig type definition ==="
cat -n frontend/api/client.ts | head -20

echo ""
echo "=== request function signature and params handling ==="
cat -n frontend/api/client.ts | sed -n '64,99p'

Repository: GenerateNU/toggo

Length of output: 2324


🏁 Script executed:

#!/bin/bash
# Check generated files that pass params to request
echo "=== Files in frontend/api that use params ==="
fd -t f "\\.ts$" frontend/api/ --exec grep -l "params" {} \;

echo ""
echo "=== Check useGetAllTrips.ts to see params being passed ==="
cat -n frontend/api/trips/useGetAllTrips.ts | head -30

Repository: GenerateNU/toggo

Length of output: 2973


🏁 Script executed:

#!/bin/bash
# Search for how params are handled in the request function
echo "=== Full request function implementation ==="
rg "function request|const request" -A 50 --type=ts frontend/api/client.ts | head -80

Repository: GenerateNU/toggo

Length of output: 1227


Query parameters are silently dropped — pagination will not work.

The params field is spread into the request config on line 24, but RequestConfig in frontend/api/client.ts (lines 6-12) does not define a params property. The request function (lines 65-99) constructs the fetch URL as ${BASE_URL}${config.url} without serializing query parameters. This means limit, cursor, and any other query parameters passed to getAllTrips() are ignored.

The function documentation explicitly states "Use limit and cursor query params", but they are never sent to the server. This affects 22+ generated files across the codebase (useGetTrip.ts, useGetPaginatedComments.ts, useGetUser.ts, etc.).

Add a params field to RequestConfig and serialize it into the URL in the request function. Alternatively, configure Kubb to construct the full URL with query parameters in the generated code.

🤖 Prompt for AI Agents
In `@frontend/api/trips/useGetAllTrips.ts` at line 24, The request helper is
dropping query params so pagination params passed from useGetAllTrips (and other
generated hooks) are never sent; add an optional params property to the
RequestConfig type (e.g., params?: Record<string | number, any> |
URLSearchParams) and update the request function to serialize those params onto
the fetch URL (e.g., append new URLSearchParams(config.params).toString() to
`${BASE_URL}${config.url}` only when params exist), ensuring callers like
useGetAllTrips.ts that pass params are transmitted to the server.

tripIDGroup.Delete("", tripController.DeleteTrip)

// /api/v1/trips/:tripID/pitches
tripIDGroup.Post("/pitches", pitchController.CreatePitch)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we ensure that membership is checked for all of these routes so that people who are not members cannot get a trip's pitches. i believe there is a middleware for this so we can pass the middleware in here

Copy link
Contributor

@in-mai-space in-mai-space left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some nits on checking membership of the trip, but overall looking good 🔥

can you confirm if the audio recorded get played if you play it from the presigned url?

AudioS3Key string `bun:"audio_s3_key,notnull" json:"audio_s3_key"`
Duration *int `bun:"duration" json:"duration,omitempty"`
CreatedAt time.Time `bun:"created_at,nullzero" json:"created_at"`
UpdatedAt time.Time `bun:"updated_at,nullzero" json:"updated_at"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a change that's new since Thursday, but any chance we can add a new migration for ImageURLs?

At a minimum, I'm imagining that we'll automatically populate some kind of image based on the destination (so null?), but design seems to be leaning to having the choice of uploading your own images as well. You can keep this as a nullable list of strings, we can do the s3 upload logic later.

Alternatively, if you feel this is a hefty change, we can make this a separate PR :)

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants